Skip to content

[https://nvbugs/6426852][fix] Add explicit gc.collect() between del weights and… - #16144

Open
trtllm-agent wants to merge 1 commit into
NVIDIA:mainfrom
tensorrt-cicd:repair-bot-bug6426852
Open

[https://nvbugs/6426852][fix] Add explicit gc.collect() between del weights and…#16144
trtllm-agent wants to merge 1 commit into
NVIDIA:mainfrom
tensorrt-cicd:repair-bot-bug6426852

Conversation

@trtllm-agent

@trtllm-agent trtllm-agent commented Jul 8, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Collaborator

Summary

  • Root cause: Per-RPC ipc_collect() couldn't reclaim IPC mappings because del weights alone left the mapped storages in unreachable cycles; many partial-update RPCs on MoE model exhausted CUDA driver's IPC handle table.
  • Fix: Add explicit gc.collect() between del weights and torch.cuda.ipc_collect() in the per-RPC branch so IPC-mapped storages are released in the same iteration, complementing the prior producer-side lazy-replica fix.
  • Automated fix generated by repair-bot

Test plan

  • Verify fix on the same GPU type as the original failure
  • Check for regressions in related tests

Links

Summary by CodeRabbit

  • Bug Fixes

    • Improved weight update reliability during repeated requests by ensuring GPU memory handles are released more consistently.
    • Reduced the chance of failures when updating model weights across multiple GPU operations.

  • Tests

    • Updated integration waivers and related GPU test behavior to reflect the new weight-handling flow.
    • Adjusted test helpers to create GPU weight copies more lazily.

@trtllm-agent
trtllm-agent requested a review from a team as a code owner July 8, 2026 21:27
@trtllm-agent
trtllm-agent requested a review from schetlur-nv July 8, 2026 21:27
@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 448e27d6-f7eb-42c7-8225-25bccd60b1cf

📥 Commits

Reviewing files that changed from the base of the PR and between 5ce293f and 9821ab7.

📒 Files selected for processing (3)
  • tensorrt_llm/llmapi/rlhf_utils.py
  • tests/integration/test_lists/waives.txt
  • tests/unittest/_torch/ray_orchestrator/single_gpu/test_llm_update_weights.py
💤 Files with no reviewable changes (1)
  • tests/integration/test_lists/waives.txt
📝 Walkthrough

Walkthrough

Adds an explicit gc.collect() call before torch.cuda.ipc_collect() in the weight update path to release CUDA IPC storages. Removes an associated test waiver and refactors a test helper to lazily replicate model weights per device instead of eagerly copying to all devices.

Changes

CUDA IPC weight update fix

Layer / File(s) Summary
gc.collect() before IPC sweep
tensorrt_llm/llmapi/rlhf_utils.py		 Adds an unconditional gc.collect() call after weight reload and before torch.cuda.ipc_collect() to ensure IPC storages are released.
Lazy weight replication and waiver removal
tests/unittest/_torch/ray_orchestrator/single_gpu/test_llm_update_weights.py, tests/integration/test_lists/waives.txt		 Test helper now only clones weights for the owning device and lazily populates other devices' weights on demand; removes the obsolete waiver for the multi-gpu update-weights test.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: brb-nv, shuyixiong, mingyangHao

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the main fix: adding explicit gc.collect() for CUDA IPC handle cleanup, though it is truncated.
Description check ✅ Passed The description covers summary, root cause, fix, and test plan, but it omits the template's explicit Description and PR Checklist sections.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@trtllm-agent
trtllm-agent force-pushed the repair-bot-bug6426852 branch from 9821ab7 to ff85e5d Compare July 13, 2026 09:39
@trtllm-agent
trtllm-agent force-pushed the repair-bot-bug6426852 branch from ff85e5d to 8a94d16 Compare July 20, 2026 11:49
@trtllm-agent
trtllm-agent requested review from a team as code owners July 20, 2026 11:49
@mikeiovine
mikeiovine requested review from mikeiovine and removed request for schetlur-nv July 20, 2026 15:35
@trtllm-agent
[nvbugs/6426852][fix] Force GC before ipc_collect per partial-update RPC
701d5eb 
Partial-update loops on MoE models (Qwen3-30B-A3B) issue many
per-suffix update_weights RPCs. Each RPC's worker side reconstructs
CUDA IPC tensors via rebuild_cuda_tensor (cudaIpcOpenMemHandle) into
'weights', then drops the dict. Without an explicit gc.collect(),
the underlying storages (backed by IPC mappings) can linger in
unreachable cycles until the next full GC, so the following
torch.cuda.ipc_collect() finds nothing to reclaim and the CUDA
driver's IPC handle table saturates before the finalize step,
producing cudaErrorMapBufferObjectFailed on later RPCs.

Add gc.collect() between 'del weights' and 'torch.cuda.ipc_collect()'
in WorkerExtension.update_weights so IPC-mapped storages are actually
released before the CUDA IPC pool sweep in each iteration.

Signed-off-by: trtllm-agent <296075020+trtllm-agent@users.noreply.github.com>
@trtllm-agent
trtllm-agent force-pushed the repair-bot-bug6426852 branch from 8a94d16 to 701d5eb Compare July 22, 2026 14:01
allisonlim-nv
allisonlim-nv approved these changes Jul 29, 2026
View reviewed changes

@allisonlim-nv allisonlim-nv left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM; forces GC before ipc_collect() so IPC weight mappings are released in the same RPC.

@trtllm-agent

trtllm-agent commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator Author

NVBug 6426852 is closed as Bug - Fixed. The linked bug appears resolved elsewhere or for a reason that does not prove this PR is redundant. This PR should be judged on its own merits; repair-bot is not auto-closing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jieli-matrix jieli-matrix jieli-matrix approved these changes
@allisonlim-nv allisonlim-nv allisonlim-nv approved these changes
@asfiyab-nvidia asfiyab-nvidia Awaiting requested review from asfiyab-nvidia asfiyab-nvidia is a code owner automatically assigned from NVIDIA/trt-llm-runtime-devs
@brnguyen2 brnguyen2 Awaiting requested review from brnguyen2 brnguyen2 was automatically assigned from NVIDIA/trt-llm-devs
@ZhanruiSunCh ZhanruiSunCh Awaiting requested review from ZhanruiSunCh ZhanruiSunCh was automatically assigned from NVIDIA/trt-llm-infra-devs
@tburt-nv tburt-nv Awaiting requested review from tburt-nv tburt-nv was automatically assigned from NVIDIA/trt-llm-infra-devs
@mikeiovine mikeiovine Awaiting requested review from mikeiovine

Assignees

@shuyixiong shuyixiong

@trtllm-agent trtllm-agent

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@trtllm-agent @jieli-matrix @allisonlim-nv @shuyixiong